Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tls.py assertion issues. #435

Merged
merged 1 commit into from
Dec 28, 2023
Merged

Fix tls.py assertion issues. #435

merged 1 commit into from
Dec 28, 2023

Conversation

rthalley
Copy link
Contributor

  1. Some assertions had side effects and would cause a loss of
    framing if python was run with -O or -OO

  2. Some assertions should have been error checks.

@rthalley
Copy link
Contributor Author

I haven't figured out how to run the tests with -O yet, so consider this a draft. Everything passes for me without -O, and the problem my test server was having when run with -O went away. Arguably it would be good to add test coverage for the various kinds of input problems that can now cause TLS alerts, but I haven't figured out how to do those yet.

Copy link

codecov bot commented Dec 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (bc7c480) 100.00% compared to head (f8efe1f) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #435   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        25           
  Lines         4840      4864   +24     
=========================================
+ Hits          4840      4864   +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rthalley rthalley marked this pull request as draft December 27, 2023 18:50
src/aioquic/tls.py Outdated Show resolved Hide resolved
@jlaine jlaine force-pushed the asserts branch 9 times, most recently from 512df3b to b5e29c2 Compare December 28, 2023 15:02
1) Some assertions had side effects and would cause a loss of
   framing if python was run with -O or -OO

2) Some assertions should have been error checks.

Co-authored-by: Jeremy Lainé <[email protected]>
@jlaine jlaine marked this pull request as ready for review December 28, 2023 15:14
@rthalley rthalley merged commit 5772246 into aiortc:main Dec 28, 2023
29 checks passed
@rthalley rthalley deleted the asserts branch March 11, 2024 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants